Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Several important fixes + SOCKET-ADDRESS-IN-USE error #117

Merged
merged 7 commits into from
Jan 14, 2015

Conversation

ivan4th
Copy link
Contributor

@ivan4th ivan4th commented Jan 11, 2015

Some things broke after recent changes, namely, tests, cl-async-repl, connect-tcp-socket and pipe-server. Fixed what I could, couldn't fix catch-app-errors test which perhaps needs to be updated for the new error handling scheme. From what I understand, :catch-app-errors doesn't work anymore for event loop (although it's still present), and errors occurring during most cl-async functions now may be either signaled immediately or postponed and passed to event-cb when they occur later on, so one has to use both (handler-case ...) and :event-cb to handle errors properly for a cl-async call.

BTW, it's signaling, not throwing in CL, throw is a misnomer because it refers to a special operator that has nothing to do with conditions.

Also, errors aren't called 'events' in libuv, are they? Maybe :event-cb should be really :error-cb?

@ivan4th ivan4th changed the title A couple of small but important fixes Several important fixes + SOCKET-ADDRESS-IN-USE error Jan 11, 2015
@orthecreedence
Copy link
Owner

I think we may be butting heads some, and I'm now regretting not doing some of my changes in a new branch. It has been a lot of error handling updates mainly, but also some reorganization. I'll review this one carefully today and do my best to incorporate your changes with mine.

@ivan4th
Copy link
Contributor Author

ivan4th commented Jan 12, 2015

Yeah, I'm using cl-async daily and was a bit lost after pulling recent changes, sorry if I did some duplicate work. The fixes are necessary for https://github.com/ivan4th/cl-mqtt

@orthecreedence orthecreedence merged commit 5c77fcb into orthecreedence:master Jan 14, 2015
@hijarian
Copy link

@ivan4th This problem with pulling BC breaking changes reminded of the message from this article: http://grimoire.ca/git/stop-using-git-pull-to-deploy

@orthecreedence
Copy link
Owner

Ok, most of the conflicts were in the tests which was an easy fix.

Please be sure to grab the latest cl-libuv as well, I merged in the grovel branch and things seem to be running much smoother now.

@orthecreedence
Copy link
Owner

Just read your initial comment in more detail. I guess cl-async is having a bit of an identity crisis when handling errors. Please see orthecreedence/blackbird#8.

A recent addition is the restart send-to-eventcb which mimicks the old error handling when :catch-app-errors t is passed. If :catch-app-errors nil is given, all errors bubble up, but libuv "events" are handed directly to the event-cb (things like socket-eof, filesystem-enoent, etc).

So there's a distinction between libuv events (non-zero status codes passed back in a libuv callback) and errors (user-generated errors created outside of cl-async, but possibly in one of its callbacks).

The new error handling system lets all non-cl-async events flow through untouched (the old system would handler-case the error and re-throw), unless :catch-app-errors is given in which case a few things happen:

  • if :send-errors-to-eventcb t is given, any user-generated errors are trapped and passed into the eventcb of the operation the error occured in. this mimicks the behavior of the old cl-async, and is on by default
  • if :catch-app-errors is a function and :send-errors-to-eventcb is either nil or an event-cb doesnt exist for the given operation, the function given to :catch-app-errors will be used as a global handler for that error
  • if :catch-app-errors t is given and :send-errors-to-eventcb is nil, a generic method handle-error will be given the error object.

your input on how this is set up would be great. at this point, things have been changing so much that i'd sooner have a number of somewhat-breaking changes and make things a lot better than be stuck in some of the poor decisions I made in the past.

Also, tcp-connect, tcp-server, tcp-ssl-connect & tcp-ssl-server now detect whether the event-cb is given for the fourth arg and warn the user to use the :event-cb keyword arg instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants